-
Notifications
You must be signed in to change notification settings - Fork 218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixing summary calls in simulating.ipynb #239
Conversation
"pygments_lexer": "ipython3", | ||
"version": "3.4.3" | ||
"pygments_lexer": "ipython2", | ||
"version": "2.7.10" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The notebook has to be run in python 3. See spatialaudio/nbsphinx#27
54f974c
to
aa7d9db
Compare
This most recent update should hopefully address running the notebook under 3.4, (as well as fixing what seemed to be a newly depricated These changes are meant to address the main issue of #238 (model solution not being updated), which seem to be the most pressing. I'd vote to move the discussion on changes to the methods over to #240, if only because then the docs can get updated sooner. |
out_fluxes = out_fluxes.round(digits) | ||
in_fluxes.sort_values(by='x', inplace=True) | ||
in_fluxes = in_fluxes.round(digits) | ||
out_fluxes = out_fluxes.sort_values( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why this is necessary.
It looks to be less efficient than the inplace call.
Thanks! I think you can remove the changes to summary and rounding - My guess is that you are trying to fix the same bug I already fixed in 9395b83. Maybe rebase off of the latest master? |
Previously, the summary calls were made on a solution object that was optimized for ATP maintenance, while the current objective was for biomass optimization
aa7d9db
to
e7b42c1
Compare
Oops! I missed that, thanks for being on top of the version updates. Should be fixed now. |
Fixing summary calls in simulating.ipynb
Previously, the summary calls were made on a solution object that was
optimized for ATP maintenance, while the current objective was for
biomass optimization. Addresses #238